Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Assert assertions to Pester #2428

Merged
merged 63 commits into from
May 21, 2024
Merged

Add Assert assertions to Pester #2428

merged 63 commits into from
May 21, 2024

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Apr 2, 2024

Fix #1423
Fix #1316
Fix #1323
Fix #1355

PR Summary

This PR adds assertions from Assert module to Pester, to serve as a base for future assertions and to live side-by-side with the existing Should assertions that are based on parameter sets.

PR Checklist

  • PR has meaningful title
  • Summary describes changes
  • PR is ready to be merged
    • If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
  • Tests are added/update (if required)
  • Documentation is updated/added (if required)

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PSScriptAnalyzer found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@fflaten
Copy link
Collaborator

fflaten commented Apr 2, 2024

We should probably decide on #2381 soon, so these assertions can either implement it - or not. 🙂

@nohwnd nohwnd changed the base branch from main to dev/6.x.x April 10, 2024 19:15
@nohwnd
Copy link
Member Author

nohwnd commented Apr 21, 2024

/azp run

Copy link
Contributor

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@nohwnd nohwnd marked this pull request as ready for review May 18, 2024 13:06
@nohwnd
Copy link
Member Author

nohwnd commented May 18, 2024

Should be good enough to release in 6.0.0-alpha1 so we can get other people to look at this. Please give at another quick review @fflaten .

@fflaten
Copy link
Collaborator

fflaten commented May 18, 2024

With 7k linediff I can't promise "quick", but I'll try tomorrow when my browser is done rendering the diff. 🙂 I'll trust your tests for functionality.

Copy link
Collaborator

@fflaten fflaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 🚀 I left some comments, some of which should be addressed before alpha.

I haven't tested these manually and only skimmed through the tests for now. Wouldn't be able to find missing testcases on such a large PR tbh. and it looks like you've covered the basics.

src/Module.ps1 Show resolved Hide resolved
src/csharp/Pester/TimeSpanOrStringWithTimeUnit.cs Outdated Show resolved Hide resolved
src/functions/assert/Common/Collect-Input.ps1 Outdated Show resolved Hide resolved
src/functions/assert/Time/Should-BeFasterThan.ps1 Outdated Show resolved Hide resolved
src/functions/assert/Equivalence/Should-BeEquivalent.ps1 Outdated Show resolved Hide resolved
tst/Help.Tests.ps1 Show resolved Hide resolved
@nohwnd
Copy link
Member Author

nohwnd commented May 20, 2024

I've fixed all the suggestions, and what I did not fix I moved to issues :) Pls approve.

@nohwnd nohwnd merged commit b577d1b into main May 21, 2024
11 checks passed
@nohwnd nohwnd deleted the add-assert branch May 21, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants